Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

docker: allow import docker in reset to fail #268

Merged
merged 1 commit into from
Oct 3, 2024
Merged

docker: allow import docker in reset to fail #268

merged 1 commit into from
Oct 3, 2024

Conversation

guppy0130
Copy link
Contributor

@guppy0130 guppy0130 commented Sep 18, 2024 •
edited
Loading

an alternative implementation could be

for driver in drivers():
    with suppress(ImportError):
        driver.reset()

in this section: https://github.com/ansible/molecule/blob/4e2592e5d77b181133243b8b685933f6daff3897/src/molecule/command/reset.py#L49

pros of this PR's implementation:

  • scoped to just docker driver

pros of with suppress(ImportError) in molecule:

  • driver authors don't have to remember to try/except ImportError: pass in their reset()

@guppy0130 guppy0130 force-pushed the allow_imports_in_reset_to_fail branch 2 times, most recently from 4f9e84c to ab597da Compare October 1, 2024 01:01
@apatard apatard added bug Something isn't working docker labels Oct 3, 2024
@apatard
Copy link
Member

apatard commented Oct 3, 2024

seems to be working fine on local testing. It would be nice for molecule to handle this but ... it's easier to do it here I guess

@guppy0130 @apatard
docker: allow import docker in reset to fail
1c40230 
* if the user hasn't installed the `[docker]` extra, they won't have
  `docker` to `import`, so don't make `import docker` required in the
  docker driver's `reset`.
* fixes ansible-community#166
@apatard apatard force-pushed the allow_imports_in_reset_to_fail branch from ab597da to 1c40230 Compare October 3, 2024 20:11
@apatard apatard merged commit 40a62f8 into ansible-community:main Oct 3, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apatard apatard apatard approved these changes

Assignees
No one assigned
Labels
bug Something isn't working docker
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

molecule reset requires unused module
2 participants
@guppy0130 @apatard